Repository navigation
test: Assert that the file data poller goes quiet once a file is stable - #473
Merged
Merged
Conversation
…n ordinary write A plain rewrite is not atomic: the truncation, the write, and the modification-time update are separate steps, and a poll that lands between two of them legitimately reports an extra change. The poller promises generous detection, not one callback per logical change; the Reloader coalesces and skips the excess. The test that expected exactly one callback was therefore flaky on CI, and making the test's write atomic would only have hidden the behavior that real writes exercise. The write helpers write in the ordinary way again, and the test now drains the callbacks of the write and then requires silence, which still fails for a poller that does not update its baseline.
Member
|
Replaced the atomic-write approach in a63ddf4 at the request of @kinyoklion: a test that only passes when the write is atomic would hide what the poller does for ordinary writes, which is to report each observed step. The helpers write in the ordinary way again, and the test now checks the property that matters, that the poller goes quiet once the file is stable, by draining the callbacks of the write before requiring silence. The title and description are updated to match. Written by Claude Code on behalf of @kinyoklion. |
kinyoklion
approved these changes
Oct 13, 2026
This was referenced Oct 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The poller test
TestPollerFiresOncePerChangeexpected exactly one callback for one rewrite and failed on CI with "unexpected change detection" (run 38454434943). A plain rewrite is not atomic: the truncation, the write, and the modification-time update are separate steps, and a poll that lands between two of them legitimately reports an extra change. The poller promises generous detection, not one callback per logical change; the Reloader coalesces and skips the excess, and that is covered by its own tests.Making the test write atomically would hide the behavior that real writes exercise, so the write helpers write in the ordinary way. The test, renamed
TestPollerStopsFiringWhenFileIsStable, now drains the callbacks of the write and then requires silence, which still fails for a poller that does not update its baseline after a change.Local smoke testing (the poller tests repeated many times, under the race detector, under CPU contention, and with a variant whose rewrite steps are spread over several poll intervals) is in progress; the results follow in a comment.